Skip to content

Conversation

@ammar-agent
Copy link
Collaborator

Leases were leading to many errors and forcing models to re-read files repeatedly, which drains context unnecessarily.

Problem

The lease system added complexity and friction:

  • Models had to track lease values across tool calls
  • Any external file modification (IDE, git, etc.) invalidated leases
  • Failed operations required re-reading files to get new leases
  • This created error loops that consumed context tokens rapidly

Solution

Remove the lease concept entirely:

  • File operations no longer require or return leases
  • Tools read files directly when needed for editing
  • Simpler API with fewer failure modes
  • Models often fallback to bash + sed anyway when tool errors occur

Changes

  • ✅ Remove lease parameter from file_edit_replace and file_edit_insert
  • ✅ Remove lease from file_read results
  • ✅ Remove lease validation logic from all file operations
  • ✅ Update type definitions
  • ✅ Update tests (all 87 tests passing)

Why This Works

This tool environment is designed for isolated changes:

  • Each workspace is a separate git worktree
  • Changes are contained and easily reviewable
  • Models can use bash for complex file operations if needed
  • Reduces cognitive overhead for both models and developers

Generated with cmux

Leases were causing unnecessary errors, forcing models to re-read files repeatedly and draining context. Models often fallback to sed anyway, and this tool is designed to keep changes isolated within worktrees.

Changes:
- Remove lease parameter from file_edit_replace and file_edit_insert tools
- Remove lease from file_read tool results
- Remove lease validation logic from all file operations
- Update type definitions to remove lease fields
- Update and clean up tests to remove lease-related assertions
- Remove lease-specific tests that are no longer relevant

The file operations now work without lease validation, simplifying the API and reducing context drain.
@ammario ammario enabled auto-merge October 10, 2025 02:54
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

https://github.com/coder/cmux/blob/16ad58e6a433edeadb20790b7e003f63d8fd175d/src/components/Messages/ToolMessage.tsx#L53
P1 Badge Update file edit tool guards to reflect lease removal

After removing lease from file edit arguments, the type guards still require a lease property. Because the backend no longer provides or expects that field, these guards always return false and every file edit call falls through to GenericToolCall, so users lose the diff-oriented FileEditToolCall UI. This affects both replace and insert guards (the insert check has the same condition a few lines below).

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

@ammario ammario added this pull request to the merge queue Oct 10, 2025
Merged via the queue into main with commit 4f3c123 Oct 10, 2025
6 checks passed
@ammario ammario deleted the remove-file-lease branch October 10, 2025 03:13
ammar-agent added a commit that referenced this pull request Oct 10, 2025
The type guards for `file_edit_replace` and `file_edit_insert` tools were checking for the presence of `lease` in args, which is no longer part of the tool API after PR #136.

Removes the `"lease" in args` checks from:
- `isFileEditReplaceTool()` type guard
- `isFileEditInsertTool()` type guard

This allows the UI components to properly recognize and display these tool calls without the lease parameter.
sreya pushed a commit that referenced this pull request Nov 15, 2025
## Problem

After removing the lease concept from file operations in #136, the type
guards in `ToolMessage.tsx` were still checking for the presence of
`lease` in the tool arguments. This exposed a deeper architectural
issue: **hand-written type guards can drift from type definitions**.

## Root Cause

Type guards used manual property checks that TypeScript can't verify
match the actual types:

```typescript
// ❌ Problem: Can get out of sync with type definition
function isFileEditReplaceTool(toolName: string, args: unknown): args is FileEditReplaceToolArgs {
  return (
    toolName === "file_edit_replace" &&
    typeof args === "object" &&
    args !== null &&
    "file_path" in args &&
    "edits" in args &&
    "lease" in args  // ← TypeScript doesn't verify this matches FileEditReplaceToolArgs!
  );
}
```

## Solution: Schema-Driven Type Guards

Use the existing Zod schemas from `toolDefinitions.ts` as the single
source of truth:

```typescript
// ✅ Solution: Uses schema validation - always in sync
import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions";

function isFileEditReplaceTool(toolName: string, args: unknown): args is FileEditReplaceToolArgs {
  if (toolName !== "file_edit_replace") return false;
  return TOOL_DEFINITIONS.file_edit_replace.schema.safeParse(args).success;
}
```

## Benefits

1. **Single source of truth**: Zod schemas define both runtime
validation and TypeScript types
2. **Automatic sync**: Schema changes automatically propagate to type
guards
3. **More robust**: Validates full structure, not just key presence
4. **Future-proof**: Prevents entire category of drift bugs

## Changes

- Import `TOOL_DEFINITIONS` from `toolDefinitions.ts`
- Replace all manual type guards with schema-based validation
- Apply to all tools: `bash`, `file_read`, `file_edit_replace`,
`file_edit_insert`, `propose_plan`
- Net result: -33 lines of manual validation code, +13 lines of
schema-driven validation

## Related

- Depends on #136 (Remove lease concept from file operations)

_Generated with `cmux`_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants